Skip to content

Internal benchmarking/execution time for segmentation pipeline#777

Merged
m-reuter merged 4 commits intoDeep-MI:devfrom
dkuegler:feature/timeit
Jan 29, 2026
Merged

Internal benchmarking/execution time for segmentation pipeline#777
m-reuter merged 4 commits intoDeep-MI:devfrom
dkuegler:feature/timeit

Conversation

@dkuegler
Copy link
Member

@dkuegler dkuegler commented Jan 28, 2026

This PR adds a simple timing option of the segmentation pipeline that runs automatically.

This is tested in the docker image. The general intention would be to include execution time monitoring and improvements between libraries and python versions.

There are a couple of open questions to answer:

  • Should recon-surf.sh also switch over to logging execution times in a different file for clarity and standardization?
    • Not right now
  • Should this option be "hidden" behind a flag or just always active?
    • Always active

@dkuegler dkuegler marked this pull request as ready for review January 29, 2026 17:36
@dkuegler
Copy link
Member Author

--timeit and --timeit_log flags removed from run_fastsurfer.sh

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces infrastructure for measuring and logging execution time for the FastSurfer segmentation and surface pipelines, primarily via a new fs_time wrapper and a time_it helper, and wires this into run_fastsurfer.sh.

Changes:

  • Extend run_fastsurfer.sh to define an execution-time log path (via FASTSURFER_EXECTIMELOG), ensure its directory exists, and wrap major segmentation and surface commands with a time_it helper so their runtimes are logged.
  • Add a new time_it function and tighten error handling and batch-job management in recon_surf/functions.sh, including more consistent PIPESTATUS checks.
  • Replace the local fs_time implementation with a more flexible version supporting additional flags (-a, --no-load, --debug) and improved formatting of timing output.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
run_fastsurfer.sh Wires in execution-time logging: documents FASTSURFER_EXECTIMELOG, sets a default exec_time_log path, ensures its directory exists, and wraps key segmentation and surface commands with time_it, while slightly reformatting command arrays and improving surface failure messaging.
recon_surf/functions.sh Introduces time_it to run commands under fs_time into a timing file, adjusts RunIt/run_it/run_it_cmdf and RunBatchJobs for clearer status handling and logging, and keeps timecmd detection in sync with the new fs_time interface.
recon_surf/fs_time Reworks the fs_time helper with a new option parser, configurable load-logging behavior, and improved output formatting, aligning it with the new timing pipeline while maintaining compatibility with existing usage in the recon-surf tools.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…ming information is written to a new scripts/timing.log file and contains both segmentation and surface pipeline timing information.

run_fastsurfer.sh
- add additional flag --timeit
- add additional parameter --timeit_log <filename> which defaults to scripts/timing.log
- add a wrapping command that will perform the timing (wraps command in fs_time call).
- the default behavior does not change
- some minor formatting

recon_surf/functions.sh
- implements the new TimeIt function, which is the wrapper that extracts the timing information.

recon_surf/fs_time
- update and cleanup
…fastsurfer.sh

Configuration with FASTSURFER_EXECTIMELOG environment variable possible
@m-reuter m-reuter merged commit c6cea31 into Deep-MI:dev Jan 29, 2026
2 checks passed
@dkuegler dkuegler deleted the feature/timeit branch January 30, 2026 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants